Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Always respect EDGEDB_CLOUD_PROFILE when connecting to cloud #235

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

elprans
Copy link
Member

@elprans elprans commented May 9, 2023

Cloud instances, unlike local instances, do not represent local
configuration, so EDGEDB_CLOUD_PROFILE should always be respected
when connecting to such instances regardless of how compound
connection options have been specified.

@elprans elprans requested review from tailhook and fantix May 9, 2023 23:12
Copy link
Contributor

@tailhook tailhook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. But we should probably also have a shared test case to cover this issue.

@fantix
Copy link
Member

fantix commented May 13, 2023

But we should probably also have a shared test case to cover this issue.

fwiw you can revert geldata/shared-client-testcases#22 to get at least one test case for EDGEDB_CLOUD_PROFILE I think, if we decided to proceed on with this PR.

@elprans elprans changed the title Respect EDGEDB_CLOUD_PROFILE and EDGEDB_SECRET_KEY when connecting to cloud Always respect EDGEDB_CLOUD_PROFILE when connecting to cloud Oct 30, 2023
Cloud instances, unlike local instances, do not represent local
configuration, so `EDGEDB_CLOUD_PROFILE` should always be respected
when connecting to such instances regardless of how compound
connection options have been specified.
@elprans elprans requested a review from fantix October 30, 2023 20:46
elprans added a commit to geldata/shared-client-testcases that referenced this pull request Oct 30, 2023
This reverts commit 5f2453c.

`EDGEDB_CLOUD_PROFILE` is special.  See geldata/gel-rust#235.
@elprans
Copy link
Member Author

elprans commented Oct 30, 2023

But we should probably also have a shared test case to cover this issue.

fwiw you can revert edgedb/shared-client-testcases#22 to get at least one test case for EDGEDB_CLOUD_PROFILE I think, if we decided to proceed on with this PR.

Done in geldata/shared-client-testcases#29

elprans added a commit to geldata/shared-client-testcases that referenced this pull request Oct 30, 2023
This reverts commit 5f2453c.

`EDGEDB_CLOUD_PROFILE` is special.  See geldata/gel-rust#235.
@elprans elprans merged commit a109491 into master Oct 30, 2023
elprans added a commit to geldata/gel-cli that referenced this pull request Oct 30, 2023
elprans added a commit to geldata/gel-cli that referenced this pull request Oct 30, 2023
elprans added a commit to geldata/gel-cli that referenced this pull request Oct 30, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants